-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid XStream causing illegal access issues for internal JDK collections #24872
Conversation
test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java
Outdated
Show resolved
Hide resolved
test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java
Outdated
Show resolved
Hide resolved
test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java
Show resolved
Hide resolved
@snazy do you mind if I borrow your commit and only keep the tests? |
Sure! Take that one :) I suspect, this change also needs a custom converter for |
This method was deprecated in XStream 1.4.18 and in practice is a no-op
Done, PR updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woot!
We might have to add more in the future but with the infrastructure in place, that will be easy!
Thanks again for the idea! It was marvelously simple and effective |
I suppose we are ok with loosing the immutable semantics? |
That would only be a problem if folks where actually adding information into the parameters in the test methods. |
I also do think making it work in general is more important than retaining immutability. |
It's for the test method(s) parameters, so I doubt that immutability matters (a lot). |
Yeah, I think we are fine losing immutability for that. That's why I suggested this approach. |
Fixes #24492